Simplify raft component CMake logic, and allow compilation without FAISS #428
Conversation
… export set and CPM downloaded modes
…_cmake_raft_target_logic
cpp/CMakeLists.txt
Outdated
| @@ -100,96 +105,141 @@ endif() | |||
| # add third party dependencies using CPM | |||
| rapids_cpm_init() | |||
|
|
|||
| # thrust and libcudacxx need to be before cuco! | |||
| include(cmake/thirdparty/get_thrust.cmake) | |||
| # cuco before rmm to properly configure thrust/cub | |||
There was a problem hiding this comment.
@cjnolet We should restore the call to include(cmake/thirdparty/get_thrust.cmake) as the first line before cuco or rmm.
If we have cuco as the first entry it will bring in thrust 1.12 which we don't want. By having our thrust first that will fetch 1.15 and will stop cuco from fetching 1.12
There was a problem hiding this comment.
@robertmaynard, I need some help here then.
Without this change, I get a thrust cmake error when building cuml (locally) about the incorrect cub verison being picked up (1.12). With this change, cuml builds (locally) successfully and I believe it's pulling in thrust 1.15 because I would have expected to get a cub build error in HDBSCAN if it were not.
There was a problem hiding this comment.
@trxcllnt also opened a PR to cuco to update the thrust version to 1.15, which we're using in this PR: NVIDIA/cuCollections#134. I notice RMM doesn't seem to specify a thrust version: https://github.com/rapidsai/rmm/blob/branch-22.04/cmake/thirdparty/get_thrust.cmake. Is that getting set automatically by rapids cpm somehow?
There was a problem hiding this comment.
yes for rmm it uses the version specified by versions.json in rapids-cmake
| @@ -0,0 +1,9 @@ | |||
| { | |||
There was a problem hiding this comment.
I am not seeing a package_override load of this file, so it will do nothing.
…aynard/raft into refactor_cmake_raft_target_logic
|
@gpucibot merge |
The previous logic didn't allow for developers to build raft without FAISS as
raft_nnwas always required to be built.